Node22 #21
Merged
Node22 #21
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…just import order in types
|
Coverage after merging node22 into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the integrated date-fns-tz code to TypeScript v3.2.0, adds strong typing and TS/ESLint disables for compatibility, removes legacy cloneObject/assign utilities, and updates project files (package version, tests, workflows, README).
- Added TypeScript definitions, switched to named exports, and introduced new utilities (e.g.,
getTimezoneOffsetInMilliseconds). - Refactored core timezone parsing/formatting modules, replacing default exports and improving API consistency.
- Updated project metadata (version bump, changelog, CI workflows, test runner imports, README adjustments).
Reviewed Changes
Copilot reviewed 639 out of 639 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/date-fns-tz/format/formatters/index.ts | Typed and renamed exports, removed _originalDate fallback from formatters |
| src/date-fns-tz/date-fns-v2-lib/{cloneObject,assign} | Removed legacy cloneObject and assign files |
| src/date-fns-tz/_lib/tzTokenizeDate/index.ts | Introduced TS types, added fallback hackyOffset path |
| src/date-fns-tz/_lib/tzPattern/index.ts | Converted to named export with TS type |
| src/date-fns-tz/_lib/tzParseTimezone/index.ts | Refactored parsing logic, updated patterns and typings |
| src/date-fns-tz/_lib/tzIntlTimeZoneName/index.ts | Converted to named export, added hackyTimeZone, updated getDTF API |
| src/date-fns-tz/_lib/newDateUTC/index.ts | Converted to named export with TS return type |
| src/date-fns-tz/_lib/getTimezoneOffsetInMilliseconds/index.ts | Added new utility for sub-minute offsets |
| src/date-fns-tz/CHANGELOG.md | Updated to v3.2.0 changelog |
| src/date-fns-tz.spec.ts | Switched test runner to node:test and updated import paths |
| package.json | Bumped to v5.0.0, reorganized scripts, devDependencies, and engines |
| eslint.config.mjs | Added project-specific rule overrides |
| README.md | Adjusted sections and updated feature/usage notes |
| .github/workflows/*.yml | Updated Node.js versions in CI/CD pipelines |
| .env | Added sample TZ environment variable configurations |
Comments suppressed due to low confidence (4)
src/date-fns-tz/_lib/tzIntlTimeZoneName/index.ts:43
- [nitpick] The original implementation validated that
locale.codeexists and threw an error if missing. Removing that check may lead to silent failures when an invalid locale object is passed. Consider re-adding a guard to validatelocale.code.
function getDTF(
package.json:3
- The version bump to
5.0.0doesn’t align with thev3.2.0tag in the changelog. Please confirm the intended versioning scheme and update the changelog or version accordingly.
"version": "5.0.0",
src/date-fns-tz/_lib/getTimezoneOffsetInMilliseconds/index.ts:16
- [nitpick] This new function isn't referenced elsewhere; if it's public API, ensure it's documented and tested, otherwise consider removing it.
export function getTimezoneOffsetInMilliseconds(date: Date): number {
src/date-fns-tz.spec.ts:6
- Including the
.tsextension in the import path may not resolve correctly in all environments. Consider importing without the extension:from './index'.
import { tzFormat, tzUtcToZonedTime } from './index.ts';
| ) { | ||
| const timezoneOffset = getTimeZoneOffset(options.timeZone, options._originalDate || date); | ||
| X: function (date, token, options) { | ||
| const timezoneOffset = getTimeZoneOffset(options.timeZone, date); |
There was a problem hiding this comment.
The original code used options._originalDate when provided; this refactor always uses date, potentially causing incorrect offsets when options._originalDate exists. Consider reintroducing an originalDate fallback (e.g., const originalDate = options._originalDate || date) when computing the timezone offset.
Suggested change
| const timezoneOffset = getTimeZoneOffset(options.timeZone, date); | |
| const originalDate = options._originalDate || date; | |
| const timezoneOffset = getTimeZoneOffset(options.timeZone, originalDate); |
| let token: RegExpExecArray | null; | ||
| let absoluteOffset; | ||
|
|
||
| // Parse constious time zone offset formats to an offset in milliseconds |
There was a problem hiding this comment.
There's a typo in the comment: constious should be various.
Suggested change
| // Parse constious time zone offset formats to an offset in milliseconds | |
| // Parse various time zone offset formats to an offset in milliseconds |
Contributor
Author
|
please note that some of our home grown production codes in
|
ramaghanta
requested changes
Jun 17, 2025
Contributor
ramaghanta
left a comment
ramaghanta
left a comment
There was a problem hiding this comment.
just a couple of questions
| export default [ | ||
| ...checkdigitConfig, | ||
| { | ||
| rules: { |
Contributor
There was a problem hiding this comment.
Is there a way to turn these rules off only for the stuff inside date-fns and date-fns-tz directories
Contributor
Author
There was a problem hiding this comment.
good point, will try
package.json
Outdated
| "jest": { | ||
| "preset": "@checkdigit/jest-config" | ||
| "ci:style": "npm run prettier", | ||
| "ci:test": "node --env-file=.env --disable-warning ExperimentalWarning --experimental-strip-types --test-timeout 600000 --test \"src/**/*.spec.ts\"", |
Contributor
There was a problem hiding this comment.
What's different about this repo that needed --env-file=.env?
Contributor
Author
There was a problem hiding this comment.
it works the same way as process.loadEnvFile(), but to be consistent i agree it's better to do process.loadEnvFile explicitly in test suites.
carlansley
requested changes
Jun 30, 2025
Contributor
carlansley
left a comment
carlansley
left a comment
There was a problem hiding this comment.
just a few version bumps required, otherwise lftm
.github/workflows/publish-beta.yml
Outdated
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20.x' | ||
| node-version: '22.x' |
.github/workflows/publish.yml
Outdated
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20.x' | ||
| node-version: '22.x' |
README.md
Outdated
| @@ -1,87 +1,65 @@ | |||
| # Check Digit Time Library | |||
|
|
|||
| Copyright (c) 2022-2024 [Check Digit, LLC](https://checkdigit.com) | |||
package.json
Outdated
| }, | ||
| "prettier": "@checkdigit/prettier-config" | ||
| "engines": { | ||
| "node": ">=22.14" |
|
❌ PR review status - not all reviewers have approved - 1 approved - 1 outstanding |
carlansley
approved these changes
Jun 30, 2025
carlansley
approved these changes
Jun 30, 2025
|
Beta Published - Install Command: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #19
tested in the following repos: